Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[WebUI] Remove debug feature for vis.js #5994

Closed
wants to merge 2 commits into from
Closed

[WebUI] Remove debug feature for vis.js #5994

wants to merge 2 commits into from

Conversation

sarutak
Copy link
Member

@sarutak sarutak commented May 8, 2015

vis.min.js refers vis.map and this even refers vis.js which is used for debug vis.js but this debug feature is not needed for Spark itself.

This issue is really minor so I don't file this in JIRA.

/CC @andrewor14

@AmplabJenkins
Copy link

Merged build triggered.

@AmplabJenkins
Copy link

Merged build started.

@SparkQA
Copy link

SparkQA commented May 8, 2015

Test build #32168 has started for PR 5994 at commit 8be038f.

@SparkQA
Copy link

SparkQA commented May 8, 2015

Test build #32168 has finished for PR 5994 at commit 8be038f.

  • This patch fails to build.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • trait Star extends NamedExpression with trees.LeafNode[Expression]
    • trait CaseWhenLike extends Expression
    • case class CaseWhen(branches: Seq[Expression]) extends CaseWhenLike
    • case class CaseKeyWhen(key: Expression, branches: Seq[Expression]) extends CaseWhenLike

@AmplabJenkins
Copy link

Merged build finished. Test FAILed.

@AmplabJenkins
Copy link

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/32168/
Test FAILed.

@sarutak
Copy link
Member Author

sarutak commented May 8, 2015

retest this please.

@AmplabJenkins
Copy link

Merged build triggered.

@AmplabJenkins
Copy link

Merged build started.

@SparkQA
Copy link

SparkQA commented May 8, 2015

Test build #32170 has started for PR 5994 at commit 8be038f.

@JoshRosen
Copy link
Contributor

This won't actually save any space in the repo, since the old bits are going to stay around in the Git history. And it shouldn't make a difference for page load times? If that's the case, is there an issue with leaving it as-is?

@sarutak
Copy link
Member Author

sarutak commented May 8, 2015

@JoshRosen Thanks for your comment.
The purpose of this PR is not saving the space.
Actually, an effect of this issue was suggested in #5947.
If there is vis.map without vis.js (not vis.min.js), we get status code 404 when we use browser-embeded debug tools because vis.js is absent.

But in #5947, thare is a suggestion that including vis.js in Spark source tree is not reasonable so I decided to remove the usage.

@andrewor14
Copy link
Contributor

Yeah right now it actually causes a JS error if you open the developer console because vis.js is not found. Since we're not using this we might as well remove usages of it.

@SparkQA
Copy link

SparkQA commented May 8, 2015

Test build #32170 has finished for PR 5994 at commit 8be038f.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@AmplabJenkins
Copy link

Merged build finished. Test PASSed.

@AmplabJenkins
Copy link

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/32170/
Test PASSed.

@JoshRosen
Copy link
Contributor

SGTM. @andrewor14, want to merge this?

@andrewor14
Copy link
Contributor

Ok. Master 1.4.

@asfgit asfgit closed this in c45c09b May 8, 2015
asfgit pushed a commit that referenced this pull request May 8, 2015
@sarutak
[WEBUI] Remove debug feature for vis.js
1dde3b3 
`vis.min.js` refers `vis.map` and this even refers `vis.js` which is used for debug `vis.js` but this debug feature is not needed for Spark itself.

This issue is really minor so I don't file this in JIRA.

/CC andrewor14

Author: Kousuke Saruta <[email protected]>

Closes #5994 from sarutak/remove-debug-feature-for-vis and squashes the following commits:

8be038f [Kousuke Saruta] Remove vis.map entry from .rat-exclude
7404945 [Kousuke Saruta] Removed debug feature for vis.js

(cherry picked from commit c45c09b)
Signed-off-by: Andrew Or <[email protected]>
@sarutak sarutak deleted the remove-debug-feature-for-vis branch May 18, 2015 01:52
jeanlyn pushed a commit to jeanlyn/spark that referenced this pull request May 28, 2015
@sarutak @jeanlyn
[WEBUI] Remove debug feature for vis.js
be9161e 
`vis.min.js` refers `vis.map` and this even refers `vis.js` which is used for debug `vis.js` but this debug feature is not needed for Spark itself.

This issue is really minor so I don't file this in JIRA.

/CC andrewor14

Author: Kousuke Saruta <[email protected]>

Closes apache#5994 from sarutak/remove-debug-feature-for-vis and squashes the following commits:

8be038f [Kousuke Saruta] Remove vis.map entry from .rat-exclude
7404945 [Kousuke Saruta] Removed debug feature for vis.js
jeanlyn pushed a commit to jeanlyn/spark that referenced this pull request Jun 12, 2015
@sarutak @jeanlyn
[WEBUI] Remove debug feature for vis.js
7474402 
`vis.min.js` refers `vis.map` and this even refers `vis.js` which is used for debug `vis.js` but this debug feature is not needed for Spark itself.

This issue is really minor so I don't file this in JIRA.

/CC andrewor14

Author: Kousuke Saruta <[email protected]>

Closes apache#5994 from sarutak/remove-debug-feature-for-vis and squashes the following commits:

8be038f [Kousuke Saruta] Remove vis.map entry from .rat-exclude
7404945 [Kousuke Saruta] Removed debug feature for vis.js
nemccarthy pushed a commit to nemccarthy/spark that referenced this pull request Jun 19, 2015
@sarutak @nemccarthy
[WEBUI] Remove debug feature for vis.js
9b26623 
`vis.min.js` refers `vis.map` and this even refers `vis.js` which is used for debug `vis.js` but this debug feature is not needed for Spark itself.

This issue is really minor so I don't file this in JIRA.

/CC andrewor14

Author: Kousuke Saruta <[email protected]>

Closes apache#5994 from sarutak/remove-debug-feature-for-vis and squashes the following commits:

8be038f [Kousuke Saruta] Remove vis.map entry from .rat-exclude
7404945 [Kousuke Saruta] Removed debug feature for vis.js
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@sarutak @AmplabJenkins @SparkQA @JoshRosen @andrewor14